Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[python] Fix registration path with non-standard obs/var index names #2281

Merged
merged 5 commits into from
Mar 18, 2024

Conversation

johnkerl
Copy link
Member

@johnkerl johnkerl commented Mar 15, 2024

Issue and/or context: #2242

Changes: Push the kwargs all the way through the call chain

Notes for Reviewer:

@johnkerl johnkerl marked this pull request as draft March 15, 2024 15:21
Copy link

codecov bot commented Mar 15, 2024

Codecov Report

Merging #2281 (afd6d7b) into main (e294717) will increase coverage by 12.08%.
The diff coverage is n/a.

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #2281       +/-   ##
===========================================
+ Coverage   78.86%   90.94%   +12.08%     
===========================================
  Files         139       36      -103     
  Lines       10720     3845     -6875     
  Branches      215        0      -215     
===========================================
- Hits         8454     3497     -4957     
+ Misses       2168      348     -1820     
+ Partials       98        0       -98     
Flag Coverage Δ
libtiledbsoma ?
python 90.94% <ø> (ø)
r ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
python_api 90.94% <ø> (ø)
libtiledbsoma ∅ <ø> (∅)

@johnkerl johnkerl changed the title [python] Fix registration path with non-standard obs/var index names [WIP] [python] Fix registration path with non-standard obs/var index names Mar 18, 2024
@johnkerl johnkerl marked this pull request as ready for review March 18, 2024 14:50
@johnkerl johnkerl marked this pull request as draft March 18, 2024 15:12
@johnkerl
Copy link
Member Author

Back in draft; missed a test case

@johnkerl
Copy link
Member Author

I am going to split up this PR to first to a bit of refactoring on the unit-test code. Then, on top of that, the code mod within the package itself.

@johnkerl johnkerl changed the base branch from main to kerl/registration-test-neaten March 18, 2024 15:32
@johnkerl johnkerl changed the base branch from kerl/registration-test-neaten to main March 18, 2024 15:33
@johnkerl johnkerl changed the base branch from main to kerl/registration-test-neaten March 18, 2024 15:33
@johnkerl johnkerl force-pushed the kerl/registration-test-neaten branch from d0a2e30 to 619caf5 Compare March 18, 2024 15:40
@johnkerl johnkerl marked this pull request as ready for review March 18, 2024 15:58
Base automatically changed from kerl/registration-test-neaten to main March 18, 2024 18:34
Copy link
Member

@ryan-williams ryan-williams left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

couple /tmps to remove, otherwise lgtm

shutil.copy(h5ad1, "/tmp/1.h5ad")
shutil.copy(h5ad2, "/tmp/2.h5ad")
shutil.copy(h5ad3, "/tmp/3.h5ad")
shutil.copy(h5ad4, "/tmp/4.h5ad")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Delete? Per #2285 (comment)

apis/python/tests/test_registration_mappings.py Outdated Show resolved Hide resolved
@johnkerl johnkerl requested a review from ryan-williams March 18, 2024 20:20
@johnkerl
Copy link
Member Author

Thanks @ryan-williams !

@johnkerl johnkerl merged commit 7dda818 into main Mar 18, 2024
11 checks passed
@johnkerl johnkerl deleted the kerl/aov branch March 18, 2024 20:41
github-actions bot pushed a commit that referenced this pull request Mar 18, 2024
…2281)

* [python] Neaten registration-mapping test code

* [python] Fix registration path with non-standard obs/var index names

* lint

* code-review feedback
johnkerl added a commit that referenced this pull request Mar 18, 2024
…2281) (#2288)

* [python] Neaten registration-mapping test code

* [python] Fix registration path with non-standard obs/var index names

* lint

* code-review feedback

Co-authored-by: John Kerl <[email protected]>
@aaronwolen
Copy link
Member

[sc-42824]

Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants